Repository navigation
feat(runner): self-hosted observability on Cloudflare (ADR-0041, ADR-0042) - #371
Merged
Merged
Conversation
demtario
changed the title
Contributor
Contributor
…heartbeat, CI/deploy gates, alert accuracy
Nine Minor-severity findings from the observability branch's fix-cycle
triage, all in the workers/api, .github/workflows, and
workers/o11y/src/alerts scope:
1. Nightly example_daily rollup now runs under its own independent
cronStep, outside the billing chain, so an upstream throw (e.g.
reconcileBilling) can no longer skip it.
2. The API worker's five-minute cron writes one structured
{"log.kind":"cron.tick"} line (telemetry/lines.ts#logCronTickLine),
so o11y's heartbeat.lastIngest becomes a true end-to-end check even
during a real quiet period. Confirmed against normalise/otlp.ts that
no change is needed there: only an oversize record is ever dropped.
3. master.yml's three "Current Version ID:" greps are now `|| true`
guarded, so a wrangler wording change can't fail a job after the
deploy already shipped.
4. master.yml's API path gate is narrowed to containers/(live|builder)/;
deploy-o11y's own containers/o11y/ gate is unchanged.
5. ci.yml's e2e-telemetry job now runs actionlint, the AGENTS.md
dev-bypass grep and check:telemetry-leak against a production-mode
build, and a negative control proving check:telemetry-leak actually
fails against a VITE_TELEMETRY_LOCAL=1 build.
6. api-5xx-rate excludes the API's own deliberate 503s: an exact-count
subtraction (session.start's at_capacity/container_starting,
chat.answer/theme.ai's error outcome) for api/session, api/chat and
api/theme, and a route-class exclusion for the still-building
placeholder on d/:id and embed/:id (no exact count exists for that
one — a real "build failed" 500 on those two routes is also excluded,
a documented residual gap outside this task's contract ownership).
7. snapshot-jobs.ts's error line now keys the demo id hot.demo_id, the
contract's own name, not the stale demo_id.
8. api-telemetry-diagnostic.test.mjs's scratch-dir copy+import is now
wrapped in try/finally; runner/.gitignore now covers
workers/api/.hot-*.
9. Added a self-resolve test for the new-fingerprint rule (through the
real runAlerts composition, not just the rule in isolation) and a
month-rollover test for the o11y-spend-cap alert, split across
computeO11ySpend's own month-prefix LIKE read (pipeline/o11y-cost.
test.mjs) and the evaluateAndNotify fire/resolve transition
(pipeline/o11y-alerts.test.mjs).
Every fix has a test that was seen failing with the change reverted.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… leak check, 5xx rule, alert tests Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Items fixed (full detail in W1-report.md): 1. A-M5: /_internal/heartbeat no longer answered by the default fetch handler (404s on public fetch); served only through the O11yHeartbeat RPC entrypoint. 2. A-M7: deploy/Sentry hook records hash with a real per-event rawEventTime (Sentry-Hook-Timestamp header, or a bucketed receive time for deploy) instead of a fixed "", so distinct same-day events no longer collide inside the 24h dedupe window. 3. B-M1: deleted box.ts's dead `state.status === "stopping"` wake-guard branch (the real @cloudflare/containers library never sets that status) and fixed the test stub to match; the existing running/healthy branch already covers the real SIGTERM-pending window. 4. B-M8: shutdown.sh now snapshots/compares every day prefix a wake's pushed records could span (bounded by Loki's own reject_old_samples_max_age: 7d), not just today/yesterday. 5. B-M5: o11y.drain now emits reason: "reopen" when a drain batch replays reopened keys (a new one-shot ledger marker), and the contract's `value` row is corrected to describe droppedOld (the packages/runtime METRICS registry already allowed "reopen" — no change needed there). 6. T01 M3/M8: added a pinning test for [auth.anonymous] enabled=false and widened the Dockerfile secret-scan regex to cover LOKI_S3_ACCESS_KEY_ID and AE_SQL_TOKEN. 7. T03 logo dup: real single-source dedup (Worker importing the .svg asset) is not cheap here (no raw-text-import bundler convention, cross-package paths) — added a drift-guard test instead; see report for the blocked file-deletion attempt. 8. A-M9: added a route test proving the DO write actually happens before /telemetry/collect answers 2xx, and a route test proving /telemetry/collect and /telemetry/lite share one rate limiter. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…le comments - Item 2 (A-M7): add a route-level test proving index.ts's handleSentryHook actually threads the Sentry-Hook-Timestamp header through to processSentryPayload — the existing direct-call tests would stay green even if the call site silently dropped the argument. - Item 8: the shared-rate-limiter fake now budgets per key (a Map), not with one process-wide counter, which could not actually distinguish "same key" from "two different keys, coincidentally exhausted at the same total call count." Revert-verified against the regression the item names (a route-scoped key in lite.ts), not a full gate bypass. - Item 3: fixed 4 more per-test hooks.stop overrides and 2 stale comments that still referenced the deleted "stopping" wake-guard. - Item 1: corrected heartbeat.ts's own doc comments (still described the old direct-fetch path) and the W1 report's claim that o11y-watchdog.ts needs a code change — it does not; only the API worker's wrangler.jsonc binding does. - Item 6: replaced the non-standard \z regex anchor with (?![\s\S]). - Verified stop-roundtrip.mjs exercised the real, freshly-built image (not a stale one) by extracting /shutdown.sh from it directly. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… day-span, drain metric fields, ingest tests Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…et redaction, Grafana resource-proxy gate, deploy warnings, compose cleanup
Fixes the 9 findings from the focused code review of feat/runner-observability:
- A-C1 (critical): the API watchdog's O11Y service binding had no
entrypoint, so env.O11Y resolved to the o11y worker's default export
instead of the O11yHeartbeat RPC entrypoint — the default export 404s
/_internal/heartbeat since W1 moved it to RPC-only, permanently latching
the watchdog stale after one false Sentry page. Added
entrypoint: "O11yHeartbeat" to the binding, switched o11y-watchdog.ts to
call the RPC heartbeat() method directly (never .fetch()), updated the
contract/runbook docs, and added a structural cross-check test asserting
every O11Y* service binding's entrypoint is a real named export of the
o11y worker plus that the watchdog never uses fetch for the heartbeat.
- A-I1: stop-roundtrip.mjs's MinIO admin exec now restores allowFail on
failure (compose() forwards a trailing options object to sh()) and
redacts MINIO_PASSWORD/the restricted user's password from any failure
log that does print.
- A-I2: added a defense-in-depth allowlist gate for Grafana's modern
resource-proxy route (/api/datasources/uid/<uid>/resources/*), mirroring
the existing legacy-proxy gate. Live-verified against a real local
Grafana 11.4 + Loki plugin: push/flush/shutdown/otlp-ingest all 404
inside Grafana itself on this route (it only reaches the plugin's own
registered CallResource handlers), so it was not exploitable today, but
the Worker's own gate is meant to be the actual security boundary.
- B-I1: master.yml now emits ::warning:: when a deploy step's Current
Version ID can't be parsed, and the o11y deploy-event ingest path marks
an empty cf_version_id in both a console.warn and the record body
(never rejecting the event — the deploy already shipped).
- B-I2: dev.mjs now tears down (docker compose down, no -v) minio/
clickhouse when `docker compose up --wait` itself fails, closing the gap
the earlier startup-failure cleanup fix didn't cover.
- B-I3: e2e-o11y-local.yml's PR path filter now also covers
runner/workers/api/** and runner/packages/**, which its specs exercise.
- B-4: ci-workflow.test.mjs's actionlint test now pins the real `uses:`
step instead of matching a comment that happens to contain the word.
- B-9: dev.mjs now checks Docker availability before --reset-local-db
wipes local D1 state.
- A-M1: stop-roundtrip.mjs now defaults to its own COMPOSE_PROJECT_NAME
("o11y-stop-roundtrip"), never colliding with the dev-stack default or
compose.yml's documented manual/T01 convention.
Every fix ships with a test that fails when the fix is reverted (verified
by hand for each). Full suite: 2062/2062 pass (2046 baseline + 16 new).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…1 redaction, live-corrected A-I2 finding, A-M1 hard refusal, testable compose bring-up Fix-up round after an advisor review of the first fix commit (5f3374b): - CRITICAL SAFETY BUG: the B-9 test's cleanup unconditionally rmSync'd workers/api/.wrangler/state/v3/d1 — real local D1 state for this worktree, not a temp dir. Now records whether it pre-existed and only removes what the test itself added. - A-I1: the redaction fix was previously dead code — allowFail:true at the one credential-bearing call site meant scrubSecrets never ran, and nothing tested it. Extracted scrubSecrets into containers/o11y/local/ redact.mjs (stop-roundtrip.mjs runs main() at module scope, so it can't be imported directly from a test), dropped allowFail there so a real failure still prints a diagnostic instead of nothing, made sh() scrub MINIO_USER too (the doc comment already claimed this; the code didn't), and added real unit tests for scrubSecrets plus a structural test that the wiring is correct. - A-I2: the live check originally only hit raw Grafana, and mischaracterized the mechanism as "reaches only registered plugin handlers, not a verbatim forward". Re-verified with docker compose logs: the Loki plugin actually forwards every resource name to real Loki as /loki/api/v1/<name> — proven via the box's own container logs (resourcePath field). push/flush/ shutdown/config are unreachable only because Loki's real admin endpoints live outside that URL prefix (a property of Loki's own layout, not Grafana's routing), and push independently gets a real 405 from Loki. A --path-as-is `../` traversal at the drain's own ingest path did not escape the prefix (Loki's own router doesn't resolve dot-segments either). Corrected box.ts's and the test file's comments to state this precisely; the code fix (the allowlist gate) was already correct and unchanged — this route is a real, working forward into Loki's read API, which is exactly why the gate matters. - A-M1: a differing default doesn't protect a developer with COMPOSE_PROJECT_NAME=o11y-dev already exported — added a hard refusal when the resolved project name matches dev.mjs's own dev-stack default, with a CLI-level test. - B-I2 (should-fix): extracted the compose-up-with-teardown logic into dev-lib.mjs's bringUpO11yCompose (injectable execFileSyncImpl, mirroring resetO11yLocalState's own pattern), replacing the CLI-level test that spawned real wrangler D1 migrations just to reach the compose section (contradicting dev-script.test.mjs's own "no real wrangler/docker/vite" header) with a fast, stubbed unit test. Reverted the stub-bin/docker ps/compose additions this test needed, since nothing else uses them. - Fixed a stale heartbeat.ts comment that still said the watchdog used fetch() and that o11y-watchdog.ts "was not changed" (both now false after A-C1). Updated docs/run-and-deploy.md's known-gaps entry and removed a dangling "see A-C1 in the fix history" doc reference. pnpm test: 2068/2068 pass. typecheck, both wrangler --dry-run, actionlint, check-test-presence, and stop-roundtrip.mjs under compose (port block 7400-7405, project o11y-x1, self-torn-down) all green. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…n, Grafana resource-proxy gate, deploy warnings, compose cleanup Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…int regex linear, truncate free text before scrubbing The controller's final-review finding Z-A-C1: an anonymous POST /telemetry/collect could burn the whole 120s cpu_ms budget with about 1KB of gzip upload, because several scrub/redact/fingerprint regexes were quadratic on adversarial input with no closing delimiter (measured: redactEmailInText alone cost 4.3s at 80k chars, ~36s projected at the 256KB record cap). Fixed patterns (all made linear by bounding their previously-unbounded quantifiers, per RFC limits where one applies — local part <=64, domain <=253, DNS label <=63): - EMAIL_PATTERN and USER_AGENT_PATTERN (workers/o11y/src/normalise/text-scrub.ts) - redactPreviewHosts (packages/runtime/src/monitor.ts) - normalizeMonitorMessage's "is not defined" identifier rule (monitor.ts) Added truncation as defense in depth, independent of each pattern also being linear: every free-text string is now bounded to SCRUB_TEXT_MAX_CHARS (= INBOX_RECORD_MAX_BYTES, the existing 256KB record cap) before any scrub regex sees it (scrub.ts's scrubText/redactStringsDeep, text-scrub.ts's scrubBodyText/scrubAttributeValues), and normalizeMonitorMessage separately bounds its own input to 4096 chars before its replace-chain runs, since its output is sliced to 200 chars regardless. The 256KB choice keeps the existing "records over 256KB are dropped" oversize tests passing unchanged (pipeline/o11y-normalise.test.mjs). Documented the truncation cap in docs/observability-contract.md §8. New tests (pipeline/o11y-redos.test.mjs): a wall-clock budget (well under 100ms) on adversarial 200k-character input for every fixed pattern, the full processFaroBody pipeline, and a real worker.fetch /telemetry/collect call with a gzip body — the finding's own repro shape. Every one of these 9 tests was confirmed FAILING (timing out its budget: 1.9s-52.6s measured) against the pre-fix regexes via a targeted git stash of only the fix source files, then restored and re-verified green (now 3-43ms each). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ut of drain-time symbolication The controller's final-review finding Z-B-C1: source-map-js#originalPositionFor throws TypeError "Line must be greater than or equal to 1, got 0" on a lineno: 0 stack frame. Ingest never rejects that shape, so one anonymous POST /telemetry/collect with a crafted lineno: 0 exception frame made drain-time symbolication throw, uncaught, out of drainKey and drainBatch — stalling the drain for both tenants, since the poisoned key stayed `written` and the same deterministic-order batch replayed and threw again on every wake (about every 10 minutes, indefinitely, until R2's 7-day inbox lifecycle finally deleted the object). Three independent guard layers, per the task's own "guard the frame / guard the record / isolate each key" breakdown: 1. workers/o11y/src/drain/symbolicate.ts#resolveBody: skip a frame with line < 1 (or non-finite line/col) before ever calling originalPositionFor, and wrap that call in its own try/catch, leaving the frame byte-for-byte unresolved on any throw. 2. symbolicateResourceLogs: wrap the resolveBody call per log record in try/catch, leaving that record's body unchanged on any throw — the record-level guard the task asked for, independent of #1. 3. workers/o11y/src/drain/drain.ts#drainKey: wrap the deps.symbolicate() call in try/catch, turning a throw into a `rejected` outcome (same InboxWriterApi#rejectKey metric/alert path as any other permanent per-key failure) rather than letting it escape. drainBatch's loop only stops on an `outcome: "error"` result, so a rejected key no longer blocks the keys after it. A transient Loki/R2 failure is unaffected — those already return a LokiPushResult/null and never throw, so they keep today's error/retry semantics untouched. Optional ingest-side half (packages/runtime/src/telemetry/convert.ts #formatStackFrame): a lineno < 1 (or non-finite) now renders with no position at all, the same shape an already-missing lineno/colno gets — cheap insurance for records normalised after this fix, not a substitute for the drain-time guard (a real browser can still report this shape for reasons unrelated to any attacker). New tests, each confirmed FAILING against the pre-fix code (via a targeted git stash of only the fix source files) and passing after: - pipeline/o11y-symbolicate.test.mjs: a lineno: 0 frame (built by hand, bypassing the new ingest-side guard, so the drain-time guard is what's actually exercised) leaves the body byte-for-byte unchanged, alongside a sibling frame that still resolves normally. - pipeline/o11y-drain.test.mjs: drainKey isolates a symbolicate() throw as `rejected`; drainBatch with one poison key followed by a good key pushes the good key and rejects only the poison one; a transient Loki 503 still ends `error`, never `rejected`. - pipeline/telemetry-convert.test.mjs: formatStackFrame drops the position for lineno 0/negative/non-finite, and still renders a normal position unchanged. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ops repeat example.* pushes) faro-core's default `dedupe: true` keeps one lastPayload per API and silently skips a push that deep-equals the previous one, with no time window. A repeat Save/Download/Share (identical example.saved attrs) or a second example.open on a guide's second example never left the browser. Pass `skipDedupe: true` from the facade's event()/metric() only — pushError keeps the global dedupe default so a window.onerror/unhandledrejection storm still collapses. Adds a build+host-gated e2e-only hook (window.__t06Telemetry, stripped from production by check-telemetry-leak.mjs's SENTINELS, same guarantee as the existing __t06ReportDemoEvent/__t06SentryCapture hooks) and two e2e/telemetry-faro.spec.ts tests proving two identical example.saved/ bucket.resolve_ms pushes both reach Faro. Verified failing with the fix reverted (poll times out at 1 item, not 2). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…stack collisions across worktrees) dev.mjs's --tier=full defaulted COMPOSE_PROJECT_NAME to the fixed literal "o11y-dev", so every worktree running dev:full resolved to the SAME compose project — same containers, same named volumes. --fresh (or even a plain Ctrl-C) in one worktree could silently adopt, stop, or delete another worktree's stack. Adds one helper, dev-lib.mjs's defaultComposeProjectName(runnerRoot) — o11y-dev-<sha256(runnerRoot) prefix>, stable per worktree, distinct across worktrees — plus resolveComposeProjectName(env, runnerRoot), which keeps an explicit COMPOSE_PROJECT_NAME override winning. Used by dev.mjs (its own --tier=full default) and by stop-roundtrip.mjs's collision guard (previously a second hardcoded "o11y-dev" literal that could drift from dev.mjs's own default). o11y-dev.mjs is untouched — it never runs docker compose itself. Corrects the now-inaccurate comment in dev-lib.mjs and documents the per-worktree default (plus the one-time named-volume rename for an existing single-worktree user) in run-and-deploy.md. Tests in pipeline/dev-script.test.mjs (different roots -> different names; same root -> stable name; env override wins; drift guards on dev.mjs's and stop-roundtrip.mjs's own source) and two pre-existing pipeline/o11y-box-config.test.mjs tests updated to compare against the real function instead of a grepped literal. All verified failing with each half of the fix reverted. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The newest actionlint (kjanat/actionlint fork, v1.17.0 — the version reviewdog/action-actionlint:v1.77.0 bundles) flags every action still on `runs.using: node20` as an error, since GitHub's node20 removal date (2026-09-23) has passed. Bumped every flagged `uses:` in the three workflow files this branch touches (ci.yml, master.yml, e2e-o11y-local.yml — confirmed via `git diff origin/master...HEAD -- .github/workflows/`) to its first node24-runtime major, confirmed against each action's own action.yml: actions/checkout@v4 -> v5 actions/setup-node@v4 -> v5 actions/upload-artifact@v4 -> v6 actions/download-artifact@v4 -> v7 pnpm/action-setup@v4 -> v5 Checked each bump's release notes for behaviour changes this repo's usage hits: download-artifact v5's breaking path-behaviour change only affects `artifact-ids:` downloads (this repo always downloads by `name:` — no action needed per the action's own migration guide); setup-node v5's new automatic packageManager-based caching only activates when the `cache` input is unset (verified in the action's own source, main.ts) — every call site here already passes `cache: pnpm` explicitly, so it is unaffected. No other workflow file is touched (git diff limited to version-string changes only), and no other content in these three files changed. Verified with kjanat/actionlint v1.17.0 (downloaded release binary, scratchpad-local, not installed globally): clean (exit 0) on all three touched workflow files. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…king, exit-code capture, wording fixes
Advisor review of the Z-A-C1/Z-B-C1 fix round found one more real gap in
the same file Z-B-C1 already touches: symbolicate.ts's STACK_LINE_RE
(/^( {4}at )(.+?) \((.+?)(?::(\d+):(\d+))?\)$/) has two lazy groups
separated by a required " (" literal. It has no /g (anchored ^...$, tried
once per line), but that one attempt still backtracks catastrophically on
a line shaped like " at a (a (a (..." with no closing paren. Measured
directly: 5k chars 5ms, 10k 20ms, 20k 74ms, 40k 305ms (~4x per 2x,
quadratic) — projected to tens of seconds at the 256KB SCRUB_TEXT_MAX_CHARS
cap Z-A-C1 truncates every free-text string to, including an exception's
`value` field, which becomes the first line of the stored body and can
freely contain "\n at a (a (..." shaped text. A CPU-limit kill from
this is not a JS throw, so none of Z-B-C1's three throw-shaped guards
would catch it.
Fixed with a length guard in parseLine (MAX_STACK_LINE_LENGTH = 4096):
any line longer than a real rendered frame could ever be is skipped before
STACK_LINE_RE ever runs, same "truncate/bound before the regex touches it"
approach as the rest of this fix round.
New test (pipeline/o11y-redos.test.mjs): symbolicateResourceLogs against a
~90k-char pathological line completes well under budget and leaves the
body unresolved-but-unchanged. Confirmed failing (1480ms vs a 200ms
budget) against the pre-fix parseLine via a targeted git stash of just
symbolicate.ts, then restored and re-verified passing (~0.5-0.6ms).
Also, per the same review:
- Shrank three ReDoS-budget test inputs (redactEmailInText x2,
redactPreviewHosts) from 200k/100k to 100k/50k chars — still several
seconds over budget on the pre-fix regex, but more headroom for the
fixed linear regex under a loaded, parallel `pnpm test` run. Re-measured
under the full suite: worst case now 35.5ms of a 100ms budget (was as
tight as 40ms of 100ms in isolation).
- docs/observability-contract.md §8: clarified that normalizeMonitorMessage
uses its own, separate 4096-char bound, not the 256KB SCRUB_TEXT_MAX_CHARS
one — the previous wording implied one cap covered both.
- drain.ts: reworded the drainKey comment that claimed a symbolicate()
throw is "classified exactly like undecodable_object" — it shares the
same rejected outcome SHAPE and metric/alert path, but (like the
existing 400-rejection path a few lines below, which already does this)
carries the underlying error's own message as `reason` rather than a
fixed token.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…n-key isolation Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…m, extra revert evidence - run-and-deploy.md's H2 note claimed the old shared o11y-dev volumes "rename once" on the new per-worktree default — false: docker does not rename volumes. Corrected to describe the actual behavior (old volumes orphaned, new project starts empty, detectO11yStateDivergence's warning fires on the first run if the ledger has committed keys) and the manual cleanup command for the orphaned volumes. Updated the matching drift test. - Added the two H2 reverts the previous round's report claimed but never ran: defaultComposeProjectName ignoring runnerRoot (fails "two different roots give two different names"), and resolveComposeProjectName dropping its env short-circuit (fails "env override wins"). Both confirmed failing then restored to green. - Verified the H1 metric-only revert independently (previously only the combined revert was run, which the serial describe block short-circuited before the metric test executed): reverting just pushMeasurement's skipDedupe fails "two identical bucket.resolve_ms measurements" on its own (1 item, not 2). Restored, both H1 tests green again. - Added a behavioural companion test (not just the source-grep drift guard) reproducing dev.mjs's own --tier=full wiring end to end (resolveComposeProjectName -> composeEnv -> resetO11yLocalState) and asserting the actual docker call's COMPOSE_PROJECT_NAME is this worktree's derived name. Confirmed it also fails when resolveComposeProjectName's fallback breaks. - Confirmed no other hardcoded o11y-dev/COMPOSE_PROJECT_NAME collision risk outside the reviewed files (e2e-o11y-local.yml already uses its own explicit o11y-ci override; the one e2e spec comment uses a distinct o11y-t11-e2e example — neither collides). - HELP_TEXT nit: "see defaultComposeProjectName below" doesn't apply to terminal output; spelled out the actual pattern instead. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…worktree compose project, actions off node20 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
setup-node v5 enables package-manager caching automatically when the root package.json declares packageManager; the presence job never installs pnpm, so the step failed with "Unable to locate executable file: pnpm". Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The /grafana/* route called box.containerFetch(upstream, 3000), a JS RPC method on the GrafanaBox stub. JS RPC sends a Request body as an RPC stream, and every body-bearing request proxied that way printed "Uncaught Error: ReadableStream received over RPC disconnected prematurely." in the DO. That covers each panel's POST /api/ds/query and each datasource resources POST, and it happened on completed requests, not only on client aborts. Measured in wrangler dev: 33 errors for the 33 POSTs of one dashboards -> Runner overview -> Logs switch. GETs: 0 of 77. The route now calls stub.fetch(upstream). GrafanaBox.fetch() delegates to the gated containerFetch override, so the live-path block, the Loki datasource-proxy/resource allowlists and the not-running 503s all still apply. It pins the port to Grafana's 3000, because the base Container.fetch() would take cf-container-target-port from a forwarded client header, which could point it at Loki's 3100. The proxy also strips that header. After the fix: 0 errors across 37 POSTs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ees it On the stub.fetch() path from the previous commit, the body was piped from req.body. A box answer that never read the body (the live-path and Loki-allowlist refusals, the not-running 503s) left the runtime pumping the incoming body after the Worker had already sent the response. In wrangler dev, 30 of 30 refused 20 KB POSTs printed "Uncaught TypeError: Can't read from request stream after response has been sent", and some came back 500 from the dev proxy instead of the box's 404. The RPC path did not show this (0 of 30). The route now reads the body to the end first and forwards it as a buffer. Result: 0 of 30 errors, all 404. A client that drops mid-upload gets a 400 from the Worker, and the box is never called. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
containers/o11y/waking/{logo.svg,README.md.txt} were a hand-kept copy of
the LOGO_SVG constant the waking page actually serves; nothing reads them.
Remove them, their drift-guard test, and the comment pointing at them.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Two identical records in one ingest batch were both dropped while the hash was still marked seen, so the record was stored zero times and every later redelivery was refused as a duplicate. checkDuplicates now reports duplicates per occurrence (isDuplicate[], index-aligned with the input), InboxWriter filters by index, and handleCollect matches outcomes to items by index instead of a last-write-wins Map keyed by hash. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Nothing in the app pointed at /grafana/ (ADR-0043's future read-panel cutover doesn't change that it's the only place to browse logs/dashboards today), so an operator had no way to discover it. Add a plain same-tab- avoiding anchor in the /admin header, next to the existing Refresh/Editor chips, that goes straight through the o11y worker's own broker login. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…out flake V-triage F1: both e2e-o11y-local.yml jobs' beforeAll hooks spawn a Tier-2 `wrangler dev`, whose "Preparing container image(s)..." step (Sandbox image check/build) plus the authoring build plus the preview server can blow the 180s hook budget on a cold runner, passing only on Playwright's automatic retry and masking the flake as green CI. o11y-local.spec.ts never opens a Tier-2 session (it only asserts Tier-1 ClickHouse rows), so its API worker never needed the container step at all -- start it with --enable-containers=false, mirroring what the workflow already does for the o11y worker in that job. telemetry-metrics.spec.ts's Tier-2 test genuinely needs the container, so give its beforeAll hook real headroom (180s -> 360s) instead. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…riage) - The API worker's manual cron-trigger recipe still named the removed `--test-scheduled`/`/__scheduled` pair. wrangler 4.108 (this repo's pinned API-worker version) serves /cdn-cgi/handler/scheduled?cron=<pattern> instead; give both of the worker's own triggers (17 4 * * *, */5 * * * *) by name, and say plainly that crons never fire on their own under `wrangler dev`. - Note that the bot gate's HeadlessChrome filter applies locally too, with no client-side signal, so scripted local traffic needs a real Chrome UA. - Note that Grafana/Explore only reflect the last wake's drain -- data packed while the box stays awake is invisible until the next wake -- and link /admin's new Grafana button from the same section. - Correct the local clean-marker docs: the container writes the wake's `clean` marker to MinIO over S3, but the worker checks for it through Miniflare's own separate local R2 store, so a local wake that pushed data can never resolve `done:` -- it reopens and replays every wake. The existing "wiping only one half" warning implied ordinary local pushed-data wakes reach `done:`, which they don't; narrowed to the cases it actually covers. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
A keystroke whose prefix fails the pre-transpile dispatches no compile, so it did not extend the held sandpack.compile_ms and one continuous typing burst split into several points (5 in 70 s of statements). A compile error now re-arms the held point's timer: the same run sends 2 (mount + burst). The contract row says what closes a burst. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e-arm Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ttempt The page clock keeps running while the test polls for the 429 and waits after the fast-forward, so a fixed 59 s jump read the count past 60 s and failed whenever Faro's jitter drew below ~0.034 (about 3 % of runs). The jump now lands 58 s after the recorded attempt; the retry.at - first.at >= 60 000 assertion still proves the retry is not early. 30/30 unpinned, and 4/4 each with the post-429 Math.random pinned to 0, 0.01 and 0.99; the old fixed jump fails 3/3 pinned to 0.01. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…an edit burst only With one point per burst, the mount's cold compile made up a large share of sandpack.compile_ms, so the tier1-playground p95 panel read mount latency, which preview.ready_ms already measures. The mount's compile no longer sends a point; a mount that fails the pre-transpile dispatches no compile, so a compile error also ends the mount phase and the next compile counts as an edit's. The mount's compile errors still reach sandpack.compile_error. No alert or other panel reads compile_ms. The contract row and runbook say so. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Brings in the R15B CI fix, Z2X and Z2Y. The one conflict is the edit-burst test in e2e/telemetry-faro.spec.ts, where both sides abort every *.codesandbox.io host: it keeps the integration branch's regex route and this branch's version-remount wait and flushFaro check, with one comment naming both ways a live bundler would race the held relay. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
demtario
marked this pull request as ready for review
September 29, 2026 06:09
Member
Author
Manual verification on a local stackEverything below was exercised by hand (plus Playwright scripts and a few exploratory agents) against Traffic and metric reconciliationA 30-minute scripted run with several personas: browsing starters, forking and saving, share pages, Saves were then tested separately. The flow was fork, edit, Save, then close the tab after 0 ms, 200 ms, 1 s or 3 s, or stay. Each save persisted and was counted exactly once, by the API. Renaming a demo does not count as a save. Editor error telemetry
Saves that fail on the author's code
Embeds and the lite beacon15 Ingest gates
ScrubbingPlanted canaries (an email, an IP, a Pipeline, box lifecycle and symbolication
Observability budgetWith the budget set below this month's spend through
Alerts, checked end to end through the Slack capture
Grafana accessTested with the local bypass off, including one real sign-in through the Handsontable login broker with a @handsontable.com Google account.
WatchdogObserved through the API worker's watchdog state (locally the Sentry client is inert by design, so no event is sent).
Not covered locally (needs the sandbox or production account)
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 84872bb. Configure here.
…rst load selectExample resets sourceLoadedRef to force the starter effect down its fetch path, but the ADR-0042 open-reason logic read that same ref as "has this page ever loaded a workspace" — so every later starter pick looked like the page's own first load: no example.open on a bare `/` visit (the taxonomy stayed null, making every following example.* a no-op), and entry=deep-link instead of picker after a ?example= visit. firstLoadSettledRef tracks that fact on its own, set once sourceLoaded first goes true and never cleared by selectExample. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…t first load Both failed against the reported bug: a bare `/` visit followed by a picker pick fired zero example.open events, and a ?example= deep-link followed by a picker pick kept reporting entry=deep-link on the pick instead of picker. The first test also checks that a following example.engaged reads back the picked starter's taxonomy, not a null one. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Trims the comment to one sentence of why, per the field-doc length cap. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
expect.poll resolves the instant a count is first reached, so it can't prove "exactly one" against a later double-fire, and a bare wait before an absence check races Faro's own send buffering. Both new tests now force a flush (the established flushFaro probe) before every open/engaged count, matching the rigor the "none on re-render" check above already uses. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
A network-level throw from requestAnswer's fetch skipped the chat.answer point because the emit lived inside the ChatUnavailableError-only branch. Move it out, mirroring the theme.ai catch below, so contract §5's "a point on every outcome" holds for chat.answer too. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…t failed Save, Fork and Share/embed share one error dialog, whose title was hardcoded to "Couldn't save" even when a Fork or Share/embed request failed. Track which action set saveError and title the dialog "Couldn't save" / "Couldn't fork" / "Couldn't share" accordingly. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…able §F.3's alert signal table and threshold list covered every cron rule in workers/o11y/src/alerts/rules.ts except snapshot-build-failed-rate, though contract §5 already documents it. Add it to both, with its real threshold and window (>50% failed per framework over 30 min, at least 10 failed). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Drain-time symbolication resolves maps with @jridgewell/trace-mapping, not source-map-js, but convert.ts and two test-file comments still described the drain-time throw as source-map-js's. Point them at the library actually in use. source-map-js mentions that build test source maps (a workers/o11y devDependency) are unchanged. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
exampleSavedAttrs's doc read as if only the editor's telemetry gate controls the example.saved count. What actually gates it (contract §5) is a successful save whose request carries a valid exampleHtMajor, whoever sends it — an API-token caller can send it and is counted the same way the editor is. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The chat-answer-network-error test's header described the bug it guards against with "guard used to only emit the point" — reword to state what the guard must do now, not its history. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
§F.3's alert signal list covered every cron rule in workers/o11y/src/alerts/rules.ts except litellm-error-rate, whose threshold was already in the paragraph below (LiteLLM errors above 5%) but whose name was missing from the row. alert-eval-error stays out: its own doc comment states it is a synthetic rule runAlerts builds from every other rule's failures, not an ADR §F.3 signal. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This was referenced Sep 29, 2026
Merged
Issues attributed to commits in this pull requestThis pull request was merged and Sentry observed the following issues:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
The runner reports faults through Sentry and nothing else. There are no traces, no metric history and no browser performance data, and every number the team looks at is a D1 counter rendered by
/admin. This PR implements ADR-0041 (observability on Cloudflare) and ADR-0042 (example analytics). The shared names, Analytics Engine slots and payload shapes are frozen inrunner/docs/observability-contract.md.What ships:
handsontable-demos-o11y, a new Worker (runner/workers/o11y/) that owns/telemetry/*and/grafana/*. The ingest routes arecollect(Faro),lite(beacon),v1/logs(Cloudflare OTLP export),deploy(CI) andhooks/sentry, and each one is gated: host/env, bot filter, size and item caps, rate limit,x-o11y-secret, GitHub OIDC, Sentry HMAC, or (for/grafana/*) a signed session cookie from the Handsontable login broker. Records are normalised to OTLP, scrubbed, hashed for dedupe and committed to theInboxWriterDurable Object before the 2xx. They are then packed into EU R2 objects per tenant while the box sleeps.runner/containers/o11y/,GrafanaBoxContainer,standard-1, EU). It wakes only for a backlog or a signed-in Grafana visit. A ledgered drain pushes inbox objects to Loki, and a stop protocol writes a clean-shutdown marker only after the TSDB index is really uploaded. An unclean stop re-opens and replays the wake's keys.runner_events), written by the API worker and by the o11y worker at ingest. The alert rules are evaluated outside the box on a*/10cron, with fire-once/resolve-once state and a Slack notifier. The API worker's*/5cron adds pool and budget gauges and a watchdog on the o11y heartbeat.session.*,container.boot_ms,snapshot.build,chat.*,serve.*and more.observabilityblock, withinvocation_logs: falseand logs exported to theo11y-logsdestination.x-hot-sessionon every API fetch, and a local-only telemetry path with a post-build leak check./dand/embed(errors, plus web vitals sampled at 10%), under a 2 KB payload.example.*events,example_daily(D1 migrations0008and0009, the latter adding thedownloadedcolumn) with a nightly rollup, and the Examples & features dashboard.cf.ray/session.id/demo-id search, error panels, Sentry issue events, and the top error fingerprints. Signed-in users also get Grafana Explore for ad-hoc LogQL; dashboards stay provisioned and read-only. Every emitted metric now has a panel.e2e-o11y-localworkflow runs the Docker-backed local end-to-end specs. It triggers on demand, nightly, and on PRs that touch the o11y paths. A path-gateddeploy-o11yjob is ordered beforedeploy-api, and source maps uploaded to Sentry and to the maps bucket with a bucket-scoped key. Deploy events use GitHub OIDC. The leak checks are wired into CI./adminuses; no Cloudflare Access application. The o11y Worker verifies the broker token once and issues its own signed__Host-session cookie. The session lasts no longer than the broker token, and a request that isn't signed in never wakes the box.pnpm dev(Tier 1),pnpm dev:live(plus the API worker, Tier-2 containers and local D1 migrations),pnpm dev:full(plus the o11y worker, compose, telemetry wiring and a local Slack capture server). Local Loki and ClickHouse data persists across restarts;--freshwipes all local o11y state together. The script adopts an already-migrated local D1, pre-pulls container base images, and stops with a clear message if a pull fails.node scripts/dev.mjs --helplists the options.runner/docs/run-and-deploy.md.Sentry stays exactly as it is at merge. Both
SENTRY_SCOPE(API) andVITE_SENTRY_SCOPE(authoring) default tofull, so every report that reaches Sentry today still does. It additionally goes to the new stack. Trimming Sentry to uncaught errors is a later, separate flip with explicit preconditions in the launch plan.Merging deploys the o11y worker (
master.ymlpath gate). The one-time production setup must be done first. It is written up, self-contained, in DEV-3087: EU R2 buckets and lifecycle rules, two bucket-scoped R2 keys, the Analytics Engine token, theo11y-logsexport destination, the Worker secrets (includingO11Y_SESSION_SECRET), the WAF exception for/telemetry/*, the Slack webhook and the Sentry internal integration.ADR-0040 decisions A, B, C.2 and C.3 are superseded (C.1 stands). ADR-0043 (
/adminreads in Grafana) is not part of this PR; it follows after a week of production data.Types of changes
runner/) changeHow was this verified?
Local suite on the branch head:
pnpm test: 2412/2412 pass, 0 failures, 0 todo.pnpm -r run typecheck,pnpm --filter @handsontable/demo-authoring build,pnpm check:telemetry-leakandpnpm check:compiler-chunkall pass.wrangler deploy --dry-runpasses for the o11y, API and authoring Workers.node scripts/check-test-presence.mjs masterpasses.actionlintis clean on all three changed workflows, at the version CI's reviewdog bundles. The workflows' actions are bumped off the retirednode20runtime.e2e-o11y-localjobs.Every new test was seen failing with its change reverted.
Local end-to-end. The whole stack ran on localhost: authoring, the API worker, the o11y worker and the box under
composeandwrangler dev, driven with real traffic rather than seed data./dand/embedbeacons, fixture replay including a duplicate delivery, both crons, a watchdog fire and resolve, and a new-fingerprint alert through a Slack capture server.E2E_O11Y_LOCAL=1 pnpm e2e e2e/o11y-local.spec.ts2/2E2E_TELEMETRYFaro, metrics and example-analytics specs greencontainers/o11y/local/stop-roundtrip.mjspasses every check, including a SIGKILL negative control, a failed-listing control and a zero-ingest wakepnpm dev,dev:liveanddev:fulleach started for real, and Ctrl-C leftdocker psclean. Local data survives a restart, and--freshempties it.cf.rayandsession.id. Explore runs LogQL. A Viewer's attempt to save a dashboard is refused (403)./grafana/_o11y/callbackreturn address (302 to Google)Sandbox Cloudflare account. These throwaway probes ran on the PoCs sandbox account, never production, and every resource was deleted afterwards:
count_over_timeboth equal one clean replaymxp04)hot.demo_id,session.idandcf.rayare structured metadata onlyCriteria 3, 4 and 11 pass locally. The real Cloudflare OTLP export was captured on the sandbox: it is always JSON+gzip, the ray id arrives as
cloudflare.ray_id, and a Worker'sconsole.logJSON arrives as body text, so the ingest now parses it with an anti-spoof strip. The captured bodies were scrubbed and committed as fixtures.Still open, so ADR-0041 and ADR-0042 stay Proposed:
head_sampling_rate) as a pre-launch step.All of these are in the launch plan's post-deploy smoke.
Review. Each task had one task-scoped review and one fix round. The whole branch then had a deep review in four areas: ingest and security, box and durability, API with alerts and CI, and browser, contract and docs. It found 3 critical and 16 important issues, including:
service.nameforgery on the public routesexample_dailyrollup silently deleting daysAll of them were fixed with tests, followed by a scoped re-review, a second fix round, a security review of the broker login, and a final re-review and fix pass. That work covered:
__Host-cookies, key separation and session capping for GrafanaA focused review of the latest fix round, then one more whole-branch review looking only for Critical and High issues, found and fixed:
O11yHeartbeatRPC entrypoint after the heartbeat moved to RPC/telemetry/collectpath, now linear with timing testslineno: 0stack frame that made symbolication throw and blocked the drain queue; a failing key is now rejected on its ownexample.*eventspnpm dev:fullsharing one compose project across worktrees, so--freshin one worktree could delete another's local data/grafana/*proxied through a Durable Object RPC method, which turned every Grafana POST body into an RPC stream and loggedReadableStream received over RPC disconnected prematurelyeach time; it now goes through the box'sfetch()with a buffered bodyIndependent local verification. A separate agent drove the whole local stack with real browser traffic, fault injection and container kills. It found:
start()or the probeso11y.wakenever recording wake-to-ready timepreview.runtime_errorwithoutht_majorandcontainer.boot_ms/session.endwithout a framework, which left their panels emptyLater rounds also found:
pool.gaugecounted every 24-hour meter instead of awake sessionssnapshot.buildwas never emitted on inline builds,serve.sharecounted every API call, and thebucket.resolve_mspanels filtered on columns the metric never sets (a new dashboard-lint rule now catches that)A 30-minute traffic run then found that live typing emitted one
preview.runtime_errorper keystroke prefix (now collapsed to one per edit burst, with a scrubbed demo-runtime Loki line), and that measurement-only requests had stopped counting as accepted ingest.A symbolication run against a real hidden-sourcemap build found that drain-time symbolication had never worked in a Worker:
source-map-jsbuilds code withnew Function, which workerd forbids, and a per-frame catch hid the error. It now uses@jridgewell/trace-mapping. The test runs under--disallow-code-generation-from-strings, skipped frames log a reason, and source paths render assrc/….The last rounds found:
id, used only in the dedupe hash, and a beacon from an old cached reporter hashes exactly as beforepreview.runtime_errorinstead ofsandpack.compile_errorPOST/PATCH /api/demosanswering 500 instead of 400pnpm dev:fullhiding a stale install or a build failure behind a 120 s generic timeoutA retest then found that runtime errors typed key by key had stopped being counted at all:
example.savedis now written by the API worker when a save succeeds, so a user who leaves before the slow (8–9 s) save response is still counted. The save runs underctx.waitUntil, so the rebuild and the point survive a client disconnect. Until every deployed API marks its save response, the browser counts a save only when that marker is missing.Console warnings (Handsontable's load-time notices) no longer count as
preview.runtime_error.The final local rounds found:
api-5xx-rate. It now answers 422build_failed, with the diagnostic kept inerrorso hot-mcp still sees it, and Save, Fork and Share show the error. Exit codes 126/127 and build logs naming a network or memory failure stay 5xx. A newsnapshot-build-failed-ratealert (more than 50 % of one framework's builds failing, with at least 10 failures, over 30 min) replaces the cover the 5xx alert used to give.?payload=link recorded nothing. The server now recordspayload.boot error.console.errorcounted twice, and the Grafana gate's 429 had noRetry-After. Both are fixed.A drift test now ties the pool cap to
max_instances, and the ingest 429 carriesRetry-After.A final pass of small follow-ups added a 10 MB cap on
/grafana/*bodies, a 400 (not 500) for malformed session JSON,theme.aipoints on network failures, lite-beacon vitals sent to Analytics Engine only, and a "demo not found" message on/share.All are fixed with tests, and the kill and reload paths were re-run live. The idle stop was re-tested live: the box stopped after 15.1 idle minutes. It also confirmed replay idempotency (criterion 2, locally), the exact Loki label set (criterion 15) and the privacy canaries.
/adminnow links to Grafana.Final whole-branch review. One more Opus review of the whole diff, in four areas, reported only Critical and High issues. It found 3 Critical and 1 High, all measured, and all fixed:
/telemetry/collectrequest burn about 180 s of CPU. Every quantifier on client free text is now bounded. The ReDoS tests now include space, tab and newline runs, which the olda-run inputs missed.hot.outcome/hot.frameworkvalues became Loki stream labels, so about 30 requests could put more than 5000 streams in one inbox key and stall the drain for about 7 days. Ingest now maps both labels to closed sets, and stored browser records carryhot.outcome=none. That caps the browser tenant at 4410 streams, and the box's Loki limit is raised to 20000. A test parses the realindex_labellist and limit, so a new label can't pass unnoticed. As a backstop, a stream-limit 429 defers only its key and skips that tenant for the rest of the wake, and the other tenant keeps draining.over_capline. An unreadable inbox object defers only its own key.sandpack.compile_msis now sent once per edit burst, Faro flushes every 5 s (and still on page leave), and a 429 is retried after itsRetry-After. One tab now sends about 5 POSTs a minute.Each fix had its own focused review and fix round.
Cleanup before review. Review and verification had roughly doubled the diff, mostly with comments that narrated findings and fix rounds. An audit, then a cleanup pass, removed:
fetch()shim, a never-deployed legacy row-key migration, and a duplicated copy of the scrub helpers (the o11y worker now imports the runtime package's copy)scripts/o11y-seed.mjsseederA code-equivalence check (transpile with comments removed, then compare) proves the comment trim changed no code. Five redundant tests were merged into their neighbours, and one test was added to pin row-key padding through the real drain path.
runner/AGENTS.mdnow has a "Code comments" section so the bloat doesn't come back.Launch list. None of these blocks the merge, but each one blocks launch or the
SENTRY_SCOPEflip:SENTRY_SCOPEflip.return_tosuffix allowlist admits the Tier-2 preview hosts. The Grafana login inherits this./ddemos. Decide whether it needs its own host.@jridgewell/trace-mappinginside a real isolate. The earlier Node measurement used a library that cannot run in workerd.head_sampling_rate).PATCH /api/demos/:id, now correctly answers 400.at_capacitypath, which can only be exercised on the sandbox account.observability.logs.destinations: [], because Cloudflare can't save theo11y-logsdestination until/telemetry/v1/logsis live (its pre-flight check gets a 405). After this merges and deploys, createo11y-logs, then merge fix(runner): re-add the o11y-logs export destination (merge after the destination exists) #377, which re-adds it. Until then, the API worker's logs don't reach Loki.updated_atand ademos-apiexample.savedrow. This is proven only in unit tests so far.Checklist
runner/config/frameworks.json(see CONTRIBUTING.md); otherwise it won't appear on demos.handsontable.com. Not applicable: no example added or renamed.pnpm build(andpnpm dev) in the affected example/server-example locally. Not applicable: no example or server-example changed. The runner's own build, typecheck, tests and dry-run deploys are listed above.Related issue(s):
return_toallowlist admits Tier-2 preview hostsNote
High Risk
Adds o11y deploy jobs, production telemetry/deploy OIDC reporting, and broad authoring instrumentation on auth, errors, and analytics paths.
Overview
This PR wires self-hosted observability (ADR-0041/0042) into CI, deploy, and the authoring app—not just new workers, but gates and client instrumentation so telemetry cannot leak into prod builds unnoticed.
CI bumps core GitHub Actions and adds an
e2e-telemetryjob on every PR:actionlint, production authoring builds, dev-login andVITE_TELEMETRY_LOCALleak checks (plus a negative control), and mocked Faro/example-analytics Playwright specs. A newe2e-o11y-local.ymlruns Docker +wrangler devintegration specs on dispatch, nightly, and path-filtered PRs.e2e-live.ymlgains a local-telemetry build step for typed Sandpack compile/runtime error counting.master.ymladds path-gateddeploy-o11y(before API deploy), narrows API deploy triggers away fromcontainers/o11y/, uploads authoring source maps to R2 then strips them fromdist/, runs the same leak checks at deploy time, and posts GitHub OIDC deploy events to/telemetry/deployafter each worker ship. Authoring production builds pinVITE_SENTRY_SCOPE=full.Authoring app adds Grafana Faro behind a telemetry facade:
apiHeaders/ session headers on API calls, ADR-0042example.*events, preview metrics (bucket.resolve_ms,version.switch, edit-burst collapse for runtime errors), and Sentry scope so explicit diagnostics can go to the facade only. Admin links to Grafana and shows observability spend/cap. Supporting pieces include.env.example,DemoNotFoundfor revoked shares, and save/fork/share error dialogs.Risk: Deploy ordering, OIDC deploy reporting, and new ingest paths touch production infrastructure; client changes affect error/analytics routing across the editor.
Reviewed by Cursor Bugbot for commit d228fd1. Bugbot is set up for automated code reviews on this repo. Configure here.